Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

New json export #537

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

New json export #537

wants to merge 7 commits into from

Conversation

vincentfretin
Copy link
Collaborator

@vincentfretin vincentfretin commented May 28, 2024 •
edited
Loading

Main differences between the previous implementation and the new implementation:
Previous implementation has its own toPropString function that does its own thing and using AFRAME.utils.coordinates.stringify(propData) only for vectors instead of using stringifyComponentValue from entity.js that uses schema.stringify that have the implementation of stringify for each property types.

In the new export I'm doing the filters early while doing prepareForSerialization, that's cloning the entity (but not attached to the DOM) and then call removeAttribute if needed.
In previous implementation, it's iterating over the existing DOM, serialize each component with the "not include default values logic" that exists in entity.js but functions were copied from entity.js (not sure if there are some changes to those), and do a filter afterwards on the json with filterJSONstreet.

@netlify Netlify
Copy link

netlify bot commented May 28, 2024 •
edited
Loading

✅ Deploy Preview for 3dstreet-core-builds ready!

Name Link
🔨 Latest commit 2149566
🔍 Latest deploy log https://app.netlify.com/sites/3dstreet-core-builds/deploys/6661aa0ff97af90008c8d3c6
😎 Deploy Preview https://deploy-preview-537--3dstreet-core-builds.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

This was referenced May 28, 2024
Closed
Merged
@vincentfretin vincentfretin mentioned this pull request Aug 30, 2024
Merged
@vincentfretin
Copy link
Collaborator Author

The previous export also doesn't round the vec3, we can get
"position": "0 -1.622889224056475e-15 -0.6911566952756658"
but that's still valid.

@vincentfretin
Copy link
Collaborator Author

Other differences between the two implementations.
The old json export is setting the "primitive" key

3dstreet/src/json-utils_1.1.js

Lines 108 to 111 in afb7aa2

const geometryAttr = entity.getAttribute('geometry');
if (geometryAttr && geometryAttr.primitive) {
elemObj['primitive'] = geometryAttr.primitive;
}

and the loader read it here

3dstreet/src/json-utils_1.1.js

Lines 412 to 423 in afb7aa2

if (entityData['primitive']) {
// define a primitive in advance to apply other primitive-specific geometry properties
entity.setAttribute('geometry', 'primitive', entityData['primitive']);
}
// load this attributes in advance in right order to correctly apply other specific components
for (const attr of ['geometry', 'material']) {
if (entityData.components[attr]) {
entity.setAttribute(attr, entityData.components[attr]);
delete entityData.components[attr];
}
}

not sure why we have that delete entityData.components[attr]; line here.

example of exported json

                    {
                      "element": "a-entity",
                      "mixin": "markings solid-stripe",
                      "primitive": "plane",
                      "components": {
                        "geometry": "buffer: false; skipCache: true; width: 0.2; height: 150",
                        "material": "repeat: 1 5; transparent: true; src: #markings-atlas",
                        "scale": "1 0.4 1",
                        "rotation": "270 0 0",
                        "position": "0 0.01 0"
                      }
                    }

There is no primitive prop in geometry component, I'm not sure if this is deleted explicitly in the code, I didn't found that.
I don't know why this was implemented this way initially and what was the issue that resulted doing this implementation.

Another difference is the exported class is an array.

@vincentfretin
Copy link
Collaborator Author

old implementation has a way to remove some properties inside a component

3dstreet/src/json-utils_1.1.js

Lines 175 to 184 in afb7aa2

// a list of component:value pairs to exclude from the JSON string.
// * - remove component with any value
// "propName": {"attribute": "..."} - remove attribute from component
const removeProps = {
src: {},
normalMap: {},
'set-loader-from-hash': '*',
'create-from-json': '*',
street: { JSON: '*' }
};

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@vincentfretin